Skip to content

Conversation

ggivo
Copy link
Collaborator

@ggivo ggivo commented Oct 6, 2025

Summary

This PR renames classes from "Cluster" terminology to "MultiDb/Database" terminology to reflect the actual functionality better and improve API clarity. The changes align the naming convention across the entire multi-database failover feature.

Changes

Package Original Class Name New Class Name
redis.clients.jedis ResilientClient MultiDbClient
redis.clients.jedis MultiClusterClientConfig MultiDbConfig
redis.clients.jedis MultiClusterClientConfig.ClusterConfig MultiDbConfig.DatabaseConfig
redis.clients.jedis MultiClusterClientConfig.ClusterConfig.Builder MultiDbConfig.DatabaseConfig.Builder
redis.clients.jedis MultiClusterClientConfig.Builder MultiDbConfig.Builder
redis.clients.jedis MultiClusterClientConfig.StrategySupplier MultiDbConfig.StrategySupplier
redis.clients.jedis.builders ResilientClientBuilder MultiDbClientBuilder
redis.clients.jedis.builders ResilientClient.Builder MultiDbClient.Builder
redis.clients.jedis.mcf MultiClusterPooledConnectionProvider MultiDbConnectionProvider
redis.clients.jedis.mcf MultiClusterPooledConnectionProvider.Cluster MultiDbConnectionProvider.Database
redis.clients.jedis.mcf MultiClusterPipeline MultiDbPipeline
redis.clients.jedis.mcf MultiClusterTransaction MultiDbTransaction
redis.clients.jedis.mcf ClusterSwitchEventArgs DatabaseSwitchEvent

Impact

⚠️ Breaking Change: This is a breaking API change for users of the multi-database failover feature.

Testing

  • ✅ All existing tests updated and passing
  • ✅ Code compiles successfully
  • ✅ Maven formatter validation passes

@ggivo ggivo added the breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes. label Oct 6, 2025
Copy link

github-actions bot commented Oct 6, 2025

Test Results

   283 files  ±0    283 suites  ±0   11m 35s ⏱️ +5s
10 075 tests  - 1  9 619 ✅  - 412  456 💤 +411  0 ❌ ±0 
 2 713 runs   - 1  2 713 ✅  -   1    0 💤 ±  0  0 ❌ ±0 

Results for commit 8bd8fc1. ± Comparison against base commit 303ed10.

This pull request removes 91 and adds 90 tests. Note that renamed tests count towards both.
redis.clients.jedis.MultiDbClientTest ‑ testAddRemoveEndpointWithClusterConfig
redis.clients.jedis.mcf.CircuitBreakerThresholdsTest ‑ belowMinFailures_doesNotFailover
redis.clients.jedis.mcf.CircuitBreakerThresholdsTest ‑ minFailuresAndRateExceeded_triggersFailover
redis.clients.jedis.mcf.CircuitBreakerThresholdsTest ‑ providerBuilder_zeroRate_mapsToHundredAndHugeMinCalls
redis.clients.jedis.mcf.CircuitBreakerThresholdsTest ‑ rateBelowThreshold_doesNotFailover
redis.clients.jedis.mcf.CircuitBreakerThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean)[1]
redis.clients.jedis.mcf.CircuitBreakerThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean)[2]
redis.clients.jedis.mcf.CircuitBreakerThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean)[3]
redis.clients.jedis.mcf.CircuitBreakerThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean)[4]
redis.clients.jedis.mcf.CircuitBreakerThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean)[5]
…
redis.clients.jedis.MultiDbClientTest ‑ testAddRemoveEndpointWithDatabaseConfig
redis.clients.jedis.mcf.DatabaseEvaluateThresholdsTest ‑ belowMinFailures_doesNotFailover
redis.clients.jedis.mcf.DatabaseEvaluateThresholdsTest ‑ minFailuresAndRateExceeded_triggersOpenState
redis.clients.jedis.mcf.DatabaseEvaluateThresholdsTest ‑ providerBuilder_zeroRate_mapsToHundredAndHugeMinCalls
redis.clients.jedis.mcf.DatabaseEvaluateThresholdsTest ‑ rateBelowThreshold_doesNotFailover
redis.clients.jedis.mcf.DatabaseEvaluateThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean, boolean)[10]
redis.clients.jedis.mcf.DatabaseEvaluateThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean, boolean)[11]
redis.clients.jedis.mcf.DatabaseEvaluateThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean, boolean)[12]
redis.clients.jedis.mcf.DatabaseEvaluateThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean, boolean)[13]
redis.clients.jedis.mcf.DatabaseEvaluateThresholdsTest ‑ thresholdMatrix(int, float, int, int, boolean, boolean)[14]
…
This pull request skips 411 tests.
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpAndor
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpAndorBinary
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpAndorBinarySingleSourceShouldFail
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpAndorSingleSourceShouldFail
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpDiff
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpDiff1
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpDiff1Binary
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpDiff1BinarySingleSourceShouldFail
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpDiff1SingleSourceShouldFail
redis.clients.jedis.commands.jedis.BitCommandsTest[1] ‑ bitOpDiffBinary
…

♻️ This comment has been updated with latest results.

Copy link
Contributor

@atakavci atakavci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of names to change

@ggivo ggivo requested a review from Copilot October 7, 2025 06:58
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Refactor to rename the multi-cluster failover API to multi-database terminology for clearer alignment with actual functionality and to introduce new class and event names.

  • Renamed core classes, configs, events, and helper utilities from MultiCluster to MultiDb.
  • Updated all tests and builders to reflect new naming; added new DatabaseSwitchEvent and removed legacy ClusterSwitchEventArgs.
  • Adjusted build/test commands (Makefile) and updated configuration exposure (e.g., MultiDbConfig).

Reviewed Changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/test/java/redis/clients/jedis/scenario/ActiveActiveFailoverTest.java Updated test to use MultiDb* types and DatabaseSwitchEvent.
src/test/java/redis/clients/jedis/providers/MultiClusterProviderHealthStatusChangeEventTest.java Refactored to MultiDb naming in provider health status tests.
src/test/java/redis/clients/jedis/misc/AutomaticFailoverTest.java Switched failover tests to MultiDb API and events.
src/test/java/redis/clients/jedis/mcf/PeriodicFailbackTest.java Updated periodic failback tests to MultiDb types and helpers.
src/test/java/redis/clients/jedis/mcf/MultiDbConnectionProviderTest.java Renamed provider test; adapted to MultiDb APIs.
src/test/java/redis/clients/jedis/mcf/MultiDbConnectionProviderInitializationTest.java New initialization edge case tests for MultiDb provider.
src/test/java/redis/clients/jedis/mcf/MultiDbConnectionProviderHelper.java New helper wrapper for updated provider methods.
src/test/java/redis/clients/jedis/mcf/MultiDbConnectionProviderFailoverAttemptsConfigTest.java Updated failover attempt logic tests to MultiDb naming.
src/test/java/redis/clients/jedis/mcf/MultiDbConnectionProviderDynamicEndpointUnitTest.java Dynamic endpoint add/remove tests ported to MultiDb.
src/test/java/redis/clients/jedis/mcf/MultiDbCircuitBreakerThresholdsTest.java Circuit breaker threshold tests renamed and adjusted.
src/test/java/redis/clients/jedis/mcf/MultiClusterPooledConnectionProviderHelper.java Removed legacy helper (old MultiCluster version).
src/test/java/redis/clients/jedis/mcf/MultiClusterInitializationTest.java Removed legacy initialization test (replaced by MultiDb version).
src/test/java/redis/clients/jedis/mcf/HealthCheckTest.java Adjusted health check tests to MultiDb types and strategy supplier.
src/test/java/redis/clients/jedis/mcf/HealthCheckIntegrationTest.java Integration health check tests updated to MultiDb API.
src/test/java/redis/clients/jedis/mcf/FailbackMechanismUnitTest.java Failback unit tests migrated to MultiDb naming.
src/test/java/redis/clients/jedis/mcf/FailbackMechanismIntegrationTest.java Failback integration tests updated for MultiDb provider.
src/test/java/redis/clients/jedis/mcf/DefaultValuesTest.java Default config value tests updated to MultiDbConfig.
src/test/java/redis/clients/jedis/mcf/DatabaseEvaluateThresholdsTest.java Threshold evaluation test adapted to Database entity.
src/test/java/redis/clients/jedis/mcf/ActiveActiveLocalFailoverTest.java Active-active scenario test migrated to MultiDb constructs.
src/test/java/redis/clients/jedis/failover/FailoverIntegrationTest.java General failover integration test switched to MultiDb.
src/test/java/redis/clients/jedis/builders/UnifiedJedisConstructorReflectionTest.java Updated constructor reflection expectations to new provider name.
src/test/java/redis/clients/jedis/MultiDbClientTest.java Client API test adjusted to new config/entity naming.
src/main/java/redis/clients/jedis/mcf/MultiDbTransaction.java Renamed transaction class; updated provider/supplier references.
src/main/java/redis/clients/jedis/mcf/MultiDbPipeline.java Renamed pipeline class; updated failover connection supplier.
src/main/java/redis/clients/jedis/mcf/MultiDbFailoverBase.java Base failover abstraction renamed; updated semantics.
src/main/java/redis/clients/jedis/mcf/MultiDbConnectionSupplier.java Supplier refactored to MultiDb naming and base class.
src/main/java/redis/clients/jedis/mcf/MultiDbConnectionProvider.java Core provider refactored (maps, events, methods) to MultiDb.
src/main/java/redis/clients/jedis/mcf/MultiDbCommandExecutor.java Command executor renamed and updated to database semantics.
src/main/java/redis/clients/jedis/mcf/JedisFailoverException.java Updated messages and javadoc references to MultiDbConfig.
src/main/java/redis/clients/jedis/mcf/EchoStrategy.java Updated strategy supplier type reference.
src/main/java/redis/clients/jedis/mcf/DatabaseSwitchEvent.java New event class replacing ClusterSwitchEventArgs.
src/main/java/redis/clients/jedis/mcf/ClusterSwitchEventArgs.java Removed legacy cluster switch event args.
src/main/java/redis/clients/jedis/mcf/CircuitBreakerThresholdsAdapter.java Adapter updated for MultiDbConfig parameter.
src/main/java/redis/clients/jedis/builders/MultiDbClientBuilder.java Builder adjusted for MultiDb config and events.
src/main/java/redis/clients/jedis/UnifiedJedis.java UnifiedJedis updated to instantiate MultiDb variants.
src/main/java/redis/clients/jedis/MultiDbConfig.java Core configuration class renamed and expanded docs.
src/main/java/redis/clients/jedis/MultiDbClient.java Client facade updated to MultiDb abstractions.
pom.xml Updated spotbugs/includes path to reference MultiDbConfig.
Makefile Test phase switched from test to verify (affects lifecycle).
Comments suppressed due to low confidence (4)

src/main/java/redis/clients/jedis/mcf/MultiDbConnectionProvider.java:1

  • [nitpick] The public method name setActiveCluster no longer reflects the renamed MultiDb terminology; for API consistency with other new methods (e.g. setActiveDatabase in MultiDbClient), consider renaming this to setActiveDatabase (and providing a deprecated delegate for backward compatibility if needed).
package redis.clients.jedis.mcf;

src/main/java/redis/clients/jedis/mcf/MultiDbConnectionProvider.java:1

  • [nitpick] forceActiveCluster should be aligned with the new database terminology (e.g. forceActiveDatabase) to avoid mixed semantic usage in the public API.
package redis.clients.jedis.mcf;

src/main/java/redis/clients/jedis/mcf/MultiDbConnectionProvider.java:1

  • [nitpick] getCluster() should be renamed to getDatabase() (or retained only as a deprecated alias) for consistency with the internal Database type and the rest of the refactor.
package redis.clients.jedis.mcf;

src/main/java/redis/clients/jedis/mcf/MultiDbConnectionProvider.java:1

  • [nitpick] Method name getClusterCircuitBreaker() is inconsistent with the new MultiDb naming; suggest renaming to getDatabaseCircuitBreaker().
package redis.clients.jedis.mcf;

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@atakavci atakavci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you decided to fix those remaining old names in failover.md with #4310
this LGTM

@ggivo
Copy link
Collaborator Author

ggivo commented Oct 7, 2025

since you decided to fix those remaining old names in failover.md with #4310 this LGTM

commit addressing them in #4310

@ggivo ggivo merged commit 14356c4 into master Oct 7, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants